Stop escalating dataset delete to dataset destroy - #12639
Open
qqmyers wants to merge 17 commits into
Open
Conversation
pdurbin
reviewed
Aug 24, 2026
Co-authored-by: Philip Durbin <philip_durbin@harvard.edu>
vera
previously requested changes
Aug 25, 2026
vera
left a comment
vera
left a comment
Member
There was a problem hiding this comment.
Looks good to me.
Fixes the original issue #11253 by always restricting the "delete unpublished dataset" API to unpublished datasets (which only have a draft version and nothing else), even if called by a superuser. The restriction to unpublished datasets also keeps the API distinct from the more general "delete dataset draft" API, so there's not two duplicate APIs.
I just left a few minor textual suggestions.
Co-authored-by: Vera Clemens <16904069+vera@users.noreply.github.com>
cmbz
added
FY27 Sprint 4
Conflicts: doc/sphinx-guides/source/api/changelog.rst
pdurbin
dismissed
vera’s stale review
September 2, 2026 19:59
As discussed at standup today, Jim already made the doc changes Vera requested. Dismissing.
pdurbin
reviewed
Sep 2, 2026
pdurbin
left a comment
pdurbin
left a comment
Member
There was a problem hiding this comment.
I found a couple places where tests are failing.
I'm also wordsmithing a bit. 😅
| deleteDatasetResponse.prettyPrint(); | ||
| deleteDatasetResponse.then().assertThat() | ||
| .body("message", containsString("unpublished")) | ||
| .statusCode(FORBIDDEN.getStatusCode()); |
Member
There was a problem hiding this comment.
This is failing:
[ERROR] DatasetsIT.testCreateDataset:354 1 expectation failed.
Expected status code <403> but was <400>.
| deleteDatasetResponse.prettyPrint(); | ||
| deleteDatasetResponse.then().assertThat() | ||
| .body("message", containsString("/destroy")) | ||
| .statusCode(FORBIDDEN.getStatusCode()); |
Member
There was a problem hiding this comment.
Fail:
[ERROR] DatasetsIT.testCreateDataset:361 1 expectation failed.
Expected status code <403> but was <400>.
Co-authored-by: Philip Durbin <philipdurbin@gmail.com>
github-project-automation
Bot
moved this from In Review 🔎
to Ready for QA ⏩
in IQSS Dataverse Project
pdurbin
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it: This PR builds on #12332 to close issue #11253, removing the superuser override in the Delete Unpublished Dataset API (which allowed it to work as the Delete Published Dataset API). However, unlike the original, it keeps the restriction that Delete Unpublished Dataset only works on unpublished datasets (those that have only a draft version). The docs are updated to note this restriction and that the Delete Dataset Draft API, which deletes the draft version of any dataset, is the equivalent of Delete Unpublished Dataset when the draft is the only version.
Which issue(s) this PR closes:
Special notes for your reviewer: The error code is changed from UNAUTHORIZED (which probably should have been FORBIDDEN) to 400/BAD_REQUEST when the api is called on a published dataset.
Suggestions on how to test this: Verify that the Delete Unpublished Dataset call works when only a draft exists and that it responds with a 400 error if the dataset is published (has only published versions or if v1 exists and a draft.)
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?: included
Additional documentation: